Skip to content

Fix: Handle CRLF in syslinux and grub configs#2613

Merged
limetech merged 1 commit intounraid:masterfrom
Squidly271:patch-12
Apr 22, 2026
Merged

Fix: Handle CRLF in syslinux and grub configs#2613
limetech merged 1 commit intounraid:masterfrom
Squidly271:patch-12

Conversation

@Squidly271
Copy link
Copy Markdown
Contributor

@Squidly271 Squidly271 commented Apr 13, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Fixed boot parameter management to properly handle configuration files with Windows-style (CRLF) line endings. Improved parsing ensures correct extraction of boot labels, kernel arguments, and boot timeout settings from both syslinux and GRUB configurations. The script now works reliably with files created or edited on Windows systems.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 13, 2026

Walkthrough

This pull request updates boot parameter configuration parsing functions in a shell script to handle CRLF (DOS-style) line endings. The changes apply carriage return stripping via tr -d '\r' at the input stage across multiple parsing functions for both syslinux and GRUB configurations.

Changes

Cohort / File(s) Summary
Boot Parameter Parsing CRLF Handling
emhttp/plugins/dynamix/scripts/manage_boot_params.sh
Modified parse_append_line(), parse_grub_linux_args(), extract_grub_timeout(), validate_config(), and write_config_grub() to strip carriage returns from configuration file content before processing with awk, grep, and sed. Ensures correct label/args extraction and timeout matching when boot config files contain Windows-style CRLF line endings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Poem

🐰 Hops through configs with a careful swish,
Stripping those \rs—my shell-scripting wish!
Windows and Unix now dance in delight,
Boot parameters parsed clean and right!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: fixing CRLF handling in syslinux and grub configuration parsing, which matches the core focus of all file modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2026.04.13.2352
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2613/webgui-pr-2613.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates
  • Post-merge behavior: This preview stays available after merge until preview storage expires or it is manually cleaned up

📝 Modified Files:

Click to expand file list
emhttp/plugins/dynamix/scripts/manage_boot_params.sh

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2613, or run:

plugin remove webgui-pr-2613

🤖 This comment is automatically generated and will be updated with each new push to this PR.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
emhttp/plugins/dynamix/scripts/manage_boot_params.sh (1)

753-763: Avoid re-reading and re-normalizing the same file three times in validate_config().

At Line 754, Line 758, and Line 762, tr -d '\r' < "$cfg_file" is repeated. You can normalize once and run all checks against that result for simpler/faster validation.

♻️ Suggested simplification
-    if ! tr -d '\r' < "$cfg_file" | grep -qi "^label unraid OS$"; then
+    local normalized_cfg
+    normalized_cfg="$(tr -d '\r' < "$cfg_file")"
+
+    if ! grep -qi "^label unraid OS$" <<< "$normalized_cfg"; then
         return 1
     fi

-    if ! tr -d '\r' < "$cfg_file" | grep -qi "^label unraid OS GUI Mode$"; then
+    if ! grep -qi "^label unraid OS GUI Mode$" <<< "$normalized_cfg"; then
         return 1
     fi

-    if ! tr -d '\r' < "$cfg_file" | grep -qi "^label unraid OS Safe Mode"; then
+    if ! grep -qi "^label unraid OS Safe Mode" <<< "$normalized_cfg"; then
         return 1
     fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh` around lines 753 - 763,
In validate_config(), avoid calling tr -d '\r' < "$cfg_file" three times;
instead normalize the file once (e.g., read the file into a variable or a temp
buffer with tr -d '\r') and run the three grep -qi checks against that
normalized content for the labels "label unraid OS", "label unraid OS GUI Mode",
and "label unraid OS Safe Mode" (preserving the existing return 1 behavior if
any check fails); reference validate_config(), cfg_file, and the three label
strings to locate where to replace the repeated tr invocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh`:
- Around line 753-763: In validate_config(), avoid calling tr -d '\r' <
"$cfg_file" three times; instead normalize the file once (e.g., read the file
into a variable or a temp buffer with tr -d '\r') and run the three grep -qi
checks against that normalized content for the labels "label unraid OS", "label
unraid OS GUI Mode", and "label unraid OS Safe Mode" (preserving the existing
return 1 behavior if any check fails); reference validate_config(), cfg_file,
and the three label strings to locate where to replace the repeated tr
invocations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a55bddcd-d499-466f-8314-e5c032b656d0

📥 Commits

Reviewing files that changed from the base of the PR and between 7756908 and 6d809b8.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix/scripts/manage_boot_params.sh

@Squidly271 Squidly271 added the 7.3 label Apr 13, 2026
@Squidly271 Squidly271 requested review from SimonFair and removed request for SimonFair April 14, 2026 00:00
@limetech limetech merged commit fa21403 into unraid:master Apr 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants